Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tests for AdminNetworkPolicy #388

Merged
merged 43 commits into from
Sep 3, 2024

Conversation

tanyaveksler
Copy link
Contributor

Added some ANP tests from policy-assistant.
Fixed a small bug in handling named ports in ANP

Fixed a small bug in handling named ports in ANP
@tanyaveksler tanyaveksler self-assigned this Jul 23, 2024
@shireenf-ibm
Copy link
Contributor

hi @tanyaveksler ,

  • why the tests where added in connlist pkg and not eval ?
  • also we prefer to avoid hard-coded expected output since it will be more difficult to maintain
  • in the base branch of admin-netpols, I have already added multiple tests to the connlist_test (with test directories (of yaml resources) and expected-output files)
  • thanks for fixing the bug with named-port (fixed it in the base branch too)

@tanyaveksler
Copy link
Contributor Author

Hi @shireenf-ibm,

  • the tests were added in connlist pkg and not eval, because they test all connections for the given configuration, rather then a connection between two specific nodes.
  • regarding the hardcoded tests and expected results, this is the first draft of adding tests from policy-assistant. Since in policy-assistant the tests are hardcoded, rather than in yamls (unlike the tests that you added in the base branch of admin-netpols), we decided with @adisos to add them hardcoded as well. Also, it is in sync with what we have in the eval pkg.

@tanyaveksler tanyaveksler requested a review from adisos July 28, 2024 09:07
@adisos
Copy link
Collaborator

adisos commented Jul 28, 2024

Hi @shireenf-ibm,

* the tests were added in `connlist` pkg and not `eval`, because they test all connections for the given configuration, rather then a  connection between two specific nodes.

* regarding the hardcoded tests and expected results, this is the first draft of adding tests from policy-assistant. Since in policy-assistant the tests are hardcoded, rather than in yamls (unlike the tests that you added in the base branch of admin-netpols), we decided with @adisos to add them hardcoded as well. Also, it is in sync with what we have in the `eval` pkg.

it's okay to have the hard-coded input instead of YAMLs, but the expected output should better be in a file and not hard-coded, as this is going to be more difficult to update following changes in output..
unless the expected result is in terms of vars/structs instead of output string. in that case you can encode the expected connectivity in certain test vars, and have some code that compares expected connectivity with the one returned from the analysis function..

…s - creating pod and namespace resources per test; reading expected results from file.

Added more tests from policy assistant.
@tanyaveksler
Copy link
Contributor Author

tanyaveksler commented Jul 28, 2024

@shireenf-ibm , please look at TestANPConnectivityFromParsedResources, the last test "multiple ANPs (priority order #2)".
It has two ANPs, the first one having priority 101, that allows TCP 80-81 and UDP 80-81 for all peers.
and the second one having priority 100, that denies the same ports/protocols for all peers.
According to my understanding, the result should be that all connections are allowed (and this is what I put in expected results), but in the actual run the result is denying TCP 80-81 and UDP 80-81 connections for all peers (as if the deny policy were of a higher priority).
Do I miss something, or is it a bug?

@shireenf-ibm
Copy link
Contributor

@shireenf-ibm , please look at TestANPConnectivityFromParsedResources, the last test "multiple ANPs (priority order #2)". It has two ANPs, the first one having priority 101, that allows TCP 80-81 and UDP 80-81 for all peers. and the second one having priority 100, that denies the same ports/protocols for all peers. According to my understanding, the result should be that all connections are allowed (and this is what I put in expected results), but in the actual run the result is denying TCP 80-81 and UDP 80-81 connections for all peers (as if the deny policy were of a higher priority). Do I miss something, or is it a bug?

from the description I think the actual results are correct , for ANPs the low priority takes precedence;
so if the ANP with priority 100 denies the connections that where allowed in an ANP with priority 101, I expect a deny.
you can find more here:

Priority is a value from 0 to 1000. Policies with lower priority values have higher precedence, and are checked before policies with higher priority values.

@shireenf-ibm
Copy link
Contributor

shireenf-ibm commented Jul 29, 2024

Hi @shireenf-ibm,

* the tests were added in `connlist` pkg and not `eval`, because they test all connections for the given configuration, rather then a  connection between two specific nodes.

* regarding the hardcoded tests and expected results, this is the first draft of adding tests from policy-assistant. Since in policy-assistant the tests are hardcoded, rather than in yamls (unlike the tests that you added in the base branch of admin-netpols), we decided with @adisos to add them hardcoded as well. Also, it is in sync with what we have in the `eval` pkg.

I see, however, since multiple connlist tests are already added, and our goal is to test the new functionality in eval;
I still suggest to move these tests to the eval pkg (eval_test.go or a new test file under eval);

  • in eval_test.go we have examples with hard-coded input, including funcs for adding these hard-coded inputs to the policy-engine (for example: netpolFromYaml and podFromYaml , there are also funcs which return the hard-coded policy itself);
    these funcs can be helpful and eliminate the need to add new funcs to parser.go
  • and in order to get all conns results: you can loop the pods (see example: TestGeneralPerformance and get all conns by using pe.allAllowedConnections which also uses all new anp functions.(as if it was called from connlist)).

you may choose to write the results and compare with expected files
or instead you can add entries of "interesting" pods and expected "allowed_conns" between them.
in my opinion it is not critic to get all conns (like in connlist) for unit tests
@adisos , what do you think?

@tanyaveksler
Copy link
Contributor Author

@shireenf-ibm , please look at TestANPConnectivityFromParsedResources, the last test "multiple ANPs (priority order #2)". It has two ANPs, the first one having priority 101, that allows TCP 80-81 and UDP 80-81 for all peers. and the second one having priority 100, that denies the same ports/protocols for all peers. According to my understanding, the result should be that all connections are allowed (and this is what I put in expected results), but in the actual run the result is denying TCP 80-81 and UDP 80-81 connections for all peers (as if the deny policy were of a higher priority). Do I miss something, or is it a bug?

from the description I think the actual results are correct , for ANPs the low priority takes precedence; so if the ANP with priority 100 denies the connections that where allowed in an ANP with priority 101, I expect a deny. you can find more here:

Priority is a value from 0 to 1000. Policies with lower priority values have higher precedence, and are checked before policies with higher priority values.

OK, I see that lower priority has higher precedence. Then, another test results look incorrect. Look at the test "multiple ANPs (priority order #1)" - it is the opposite to the one described above: ANP with priority 99 allows TCP 80-81 and UDP 80-81, while ANP with priority 100 denies them. Yet, in the actual output those connections are denied.

@shireenf-ibm
Copy link
Contributor

shireenf-ibm commented Jul 30, 2024

@shireenf-ibm , please look at TestANPConnectivityFromParsedResources, the last test "multiple ANPs (priority order #2)". It has two ANPs, the first one having priority 101, that allows TCP 80-81 and UDP 80-81 for all peers. and the second one having priority 100, that denies the same ports/protocols for all peers. According to my understanding, the result should be that all connections are allowed (and this is what I put in expected results), but in the actual run the result is denying TCP 80-81 and UDP 80-81 connections for all peers (as if the deny policy were of a higher priority). Do I miss something, or is it a bug?

from the description I think the actual results are correct , for ANPs the low priority takes precedence; so if the ANP with priority 100 denies the connections that where allowed in an ANP with priority 101, I expect a deny. you can find more here:

Priority is a value from 0 to 1000. Policies with lower priority values have higher precedence, and are checked before policies with higher priority values.

OK, I see that lower priority has higher precedence. Then, another test results look incorrect. Look at the test "multiple ANPs (priority order #1)" - it is the opposite to the one described above: ANP with priority 99 allows TCP 80-81 and UDP 80-81, while ANP with priority 100 denies them. Yet, in the actual output those connections are denied.

by debugging I see that the problem is that the ANPs has no names, but when saving them in the policy-engine, I have a map from anp-name to its object; since both have the same empty ("") name string; so only the last ANP is saved in policy-engine as if it is the only ANP that was provided.
do you have the option to add names to the ANPs? (i.e. to add also metadata not just spec )

@shireenf-ibm
Copy link
Contributor

@tanyaveksler updated base branch to return error for anp objects without names

Added more tests, including BANP tests, currently commented out.
@tanyaveksler
Copy link
Contributor Author

@adisos and @shireenf-ibm: I addressed your comments, the PR is ready for your review.

// }
return res
}

func CreatePodK8sObject(pod *v1.Pod) parser.K8sObject {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should these funcs be exported?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They shouldn't :)

Copy link
Collaborator

@adisos adisos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small typos..

pkg/internal/testutils/parsed_resources_tests.go Outdated Show resolved Hide resolved
pkg/internal/testutils/parsed_resources_tests.go Outdated Show resolved Hide resolved
@tanyaveksler tanyaveksler requested a review from adisos September 3, 2024 08:12
@tanyaveksler tanyaveksler merged commit 0778788 into support_admin_netpolicy Sep 3, 2024
2 checks passed
@tanyaveksler tanyaveksler deleted the tests_for_admin_netpolicy branch September 3, 2024 08:21
shireenf-ibm added a commit that referenced this pull request Nov 28, 2024
* adding ANP to parser.k8sobj

* fixing gocritic rangeValCopy by indexing

* w.i.p. anp support - first commit

* more examples (2 ANPs/ ANP+NP)

* fixing references

* new_test that ensures rule ordering in ANP is respected

* update the conn representation as complement in case it is shorter (all but: udp 5353 instead of SCTP 1-65535,TCP 1-65535,UDP 1-5352,5354-65535)

* test with swapped rules from another test + diff test

* more-tests

* fixing conns computations and a test with multiple ANPs

* extending output formats of existing tests

* tiny fix

* fixing a tinu bug in ruleConnections func

* tiny doc updte

* tiny doc update

* a @todo tbd while review

* return error if ANPs are without name or not unique names

* remove redundant lines

* reverting the changes adding complement string representation (all but) for connectionSet

* Merge github.com:np-guard/netpol-analyzer into support_admin_netpolicy

* minor updates to netpol_errors

* currently disabling exposure-analysis when there are admin-network-policies in the input resources

* some organizations (mainly comments updates)

* updating some todo messages

* updating some todo messages/questions

* todo question

* removing a todo that had an answer for, will add some tests on that case

* fixing single anp conns compute when ingress and egress are intersected (not fully matched)

* Update pkg/internal/netpolerrors/netpol_errors.go

Co-authored-by: Tanya <[email protected]>

* Update pkg/netpol/eval/internal/k8s/adminnetpol.go

Co-authored-by: Tanya <[email protected]>

* update todo msg

* some fixes to anp so it matches latest apis

* fixing port-set union func

* Update pkg/netpol/connlist/connlist.go

Co-authored-by: Adi Sosnovich <[email protected]>

* Update pkg/netpol/eval/internal/k8s/adminnetpol.go

Co-authored-by: Adi Sosnovich <[email protected]>

* Update pkg/netpol/internal/common/connectionset.go

Co-authored-by: Adi Sosnovich <[email protected]>

* Update pkg/netpol/eval/internal/k8s/adminnetpol.go

Co-authored-by: Adi Sosnovich <[email protected]>

* Update pkg/netpol/eval/internal/k8s/adminnetpol.go

Co-authored-by: Adi Sosnovich <[email protected]>

* go.mod + lint fix

* adding todo comment

* fixes in subtract

* one line func eliminated

* uniqueness names are required for netpols and admin-netpols

* hasNetpols considers ANPs too

* Tests for AdminNetworkPolicy (#388)

* Added some ANP tests from policy-assistant.
Fixed a small bug in handling named ports in ANP

* fixing lint errors

* Fixing lint error

* Reorganized testing infrastructure from for tests fro parsed resources - creating pod and namespace resources per test; reading expected results from file.
Added more tests from policy assistant.

* fixing lint errors

* return error if ANPs are without name or not unique names

* Revert "return error if ANPs are without name or not unique names"

This reverts commit 1805549.

* Added ANP/BANP names in tests.
Added more tests, including BANP tests, currently commented out.

* Fixed lint errors.

* Fixed lint errors

* Added eval parsed resources tests (along with connlist tests).
Moved all parsed resources tests to a separate file.

* fixing lint errors

* fixing lint errors

* Added testing of CheckIfAllowed and CheckIfAllowedNew

* fixing lint errors

* making linter happy

* Reorganized eval ANP tests, to not depend on connlist.

* Small fixes.

* small fixes

* Changed expected results to not use "all but" expressions.

* making linter happy

* making linter happy

* making lint happy

* making linter happy

* make linter happy

* Creating k8sObjects during a test run, rather then in a test creation.

* making lint happy

* make lint happy

* linter

* shutting up linter

* Moved to parsed_resources_tests some functions used only there.

* Added fake pod status IP fields

* Avoiding unnecessary exports;
Fixing lint errors.

* Making linter happy

* Update pkg/internal/testutils/parsed_resources_tests.go

Co-authored-by: Adi Sosnovich <[email protected]>

* Update pkg/internal/testutils/parsed_resources_tests.go

Co-authored-by: Adi Sosnovich <[email protected]>

* Fixed typos;
removed unneeded change.

---------

Co-authored-by: shireenf-ibm <[email protected]>
Co-authored-by: Adi Sosnovich <[email protected]>

* updating some todo comment which were updated in BANP PR

* sort anps only once before allowed-conns computes (#402)

* sort anps only once before allowed-conns computes

* support_banp (#403)

* support_banp+tests

* removing lint note

* fix merge errors

* why failed to use generics for duplicated code in egressRuleSelectsPeer and ingressRuleSelectsPeer

* banp tests with swapped rules

* integrating Tanya's tests with BANP + adding results; results were compared to policy-assistant, all good

* pass action is not defined for BANP

* more code enhancement, + could not use generics

* adding banp to policy kinds

* adding comment on priority range

* Update pkg/internal/netpolerrors/netpol_errors.go

Co-authored-by: Tanya <[email protected]>

* Update pkg/netpol/eval/internal/k8s/adminnetpol.go

Co-authored-by: Tanya <[email protected]>

* Update pkg/netpol/eval/internal/k8s/adminnetpol.go

Co-authored-by: Tanya <[email protected]>

* Update pkg/netpol/eval/resources.go

Co-authored-by: Tanya <[email protected]>

* Update pkg/netpol/eval/internal/k8s/policy_connections.go

Co-authored-by: Tanya <[email protected]>

* some fixes + a new test

* tiny doc update

* demo test

* tiny change to getPoliciesSelectingPod func and deleting the "deprecated" if statements in "getAllAllowedXgressConnsFromNetpols"

* removing redundant if statements

* new parsed tests with expected outputs and a fix to the func computing "intersection" between ANP's  egress-ingress

* fixing implementing approach + some more parsed tests

* tiny doc update

* renaming func

* comment changed

* removing comment

* changing const names

* fixing if else

* code optimizations and re-org

* moving parsed_resources_tests file + some re-orgs

* optimizing collect from banp + fixing one test output

* optimize + fix + tests confirming results - tested  with policy-assistant

* deny examples parallel to the allow examples added previously

* switch

* policy conns

* collect from banp

* updating outputs with empty line at eof

* add anp_banp_blog_demo example

Signed-off-by: adisos <[email protected]>

* update example

Signed-off-by: adisos <[email protected]>

* tiny fix

* update example - add another workload and ns

Signed-off-by: adisos <[email protected]>

* update example

Signed-off-by: adisos <[email protected]>

* min-max priority consts

* moving consts

* renaming some tests + adding blog_test to the connlist_test

* test updates

* updating test

* adding references

* updating test anp_test_6_swapping_rules

* test update

* test update

* add test details

Signed-off-by: adisos <[email protected]>

---------

Signed-off-by: adisos <[email protected]>
Co-authored-by: Tanya <[email protected]>
Co-authored-by: Adi Sosnovich <[email protected]>
Co-authored-by: adisos <[email protected]>
shireenf-ibm added a commit that referenced this pull request Dec 1, 2024
* adding ANP to parser.k8sobj

* fixing gocritic rangeValCopy by indexing

* w.i.p. anp support - first commit

* more examples (2 ANPs/ ANP+NP)

* fixing references

* new_test that ensures rule ordering in ANP is respected

* update the conn representation as complement in case it is shorter (all but: udp 5353 instead of SCTP 1-65535,TCP 1-65535,UDP 1-5352,5354-65535)

* test with swapped rules from another test + diff test

* more-tests

* fixing conns computations and a test with multiple ANPs

* extending output formats of existing tests

* tiny fix

* fixing a tinu bug in ruleConnections func

* tiny doc updte

* tiny doc update

* a @todo tbd while review

* return error if ANPs are without name or not unique names

* remove redundant lines

* reverting the changes adding complement string representation (all but) for connectionSet

* Merge github.com:np-guard/netpol-analyzer into support_admin_netpolicy

* minor updates to netpol_errors

* currently disabling exposure-analysis when there are admin-network-policies in the input resources

* some organizations (mainly comments updates)

* updating some todo messages

* updating some todo messages/questions

* todo question

* removing a todo that had an answer for, will add some tests on that case

* fixing single anp conns compute when ingress and egress are intersected (not fully matched)

* Update pkg/internal/netpolerrors/netpol_errors.go

Co-authored-by: Tanya <[email protected]>

* Update pkg/netpol/eval/internal/k8s/adminnetpol.go

Co-authored-by: Tanya <[email protected]>

* update todo msg

* some fixes to anp so it matches latest apis

* fixing port-set union func

* Update pkg/netpol/connlist/connlist.go

Co-authored-by: Adi Sosnovich <[email protected]>

* Update pkg/netpol/eval/internal/k8s/adminnetpol.go

Co-authored-by: Adi Sosnovich <[email protected]>

* Update pkg/netpol/internal/common/connectionset.go

Co-authored-by: Adi Sosnovich <[email protected]>

* Update pkg/netpol/eval/internal/k8s/adminnetpol.go

Co-authored-by: Adi Sosnovich <[email protected]>

* Update pkg/netpol/eval/internal/k8s/adminnetpol.go

Co-authored-by: Adi Sosnovich <[email protected]>

* go.mod + lint fix

* adding todo comment

* fixes in subtract

* one line func eliminated

* uniqueness names are required for netpols and admin-netpols

* hasNetpols considers ANPs too

* Tests for AdminNetworkPolicy (#388)

* Added some ANP tests from policy-assistant.
Fixed a small bug in handling named ports in ANP

* fixing lint errors

* Fixing lint error

* Reorganized testing infrastructure from for tests fro parsed resources - creating pod and namespace resources per test; reading expected results from file.
Added more tests from policy assistant.

* fixing lint errors

* return error if ANPs are without name or not unique names

* Revert "return error if ANPs are without name or not unique names"

This reverts commit 1805549.

* Added ANP/BANP names in tests.
Added more tests, including BANP tests, currently commented out.

* Fixed lint errors.

* Fixed lint errors

* Added eval parsed resources tests (along with connlist tests).
Moved all parsed resources tests to a separate file.

* fixing lint errors

* fixing lint errors

* Added testing of CheckIfAllowed and CheckIfAllowedNew

* fixing lint errors

* making linter happy

* Reorganized eval ANP tests, to not depend on connlist.

* Small fixes.

* small fixes

* Changed expected results to not use "all but" expressions.

* making linter happy

* making linter happy

* making lint happy

* making linter happy

* make linter happy

* Creating k8sObjects during a test run, rather then in a test creation.

* making lint happy

* make lint happy

* linter

* shutting up linter

* Moved to parsed_resources_tests some functions used only there.

* Added fake pod status IP fields

* Avoiding unnecessary exports;
Fixing lint errors.

* Making linter happy

* Update pkg/internal/testutils/parsed_resources_tests.go

Co-authored-by: Adi Sosnovich <[email protected]>

* Update pkg/internal/testutils/parsed_resources_tests.go

Co-authored-by: Adi Sosnovich <[email protected]>

* Fixed typos;
removed unneeded change.

---------

Co-authored-by: shireenf-ibm <[email protected]>
Co-authored-by: Adi Sosnovich <[email protected]>

* updating some todo comment which were updated in BANP PR

* sort anps only once before allowed-conns computes (#402)

* sort anps only once before allowed-conns computes

* support_banp (#403)

* support_banp+tests

* removing lint note

* fix merge errors

* why failed to use generics for duplicated code in egressRuleSelectsPeer and ingressRuleSelectsPeer

* banp tests with swapped rules

* integrating Tanya's tests with BANP + adding results; results were compared to policy-assistant, all good

* pass action is not defined for BANP

* more code enhancement, + could not use generics

* adding banp to policy kinds

* adding comment on priority range

* Update pkg/internal/netpolerrors/netpol_errors.go

Co-authored-by: Tanya <[email protected]>

* Update pkg/netpol/eval/internal/k8s/adminnetpol.go

Co-authored-by: Tanya <[email protected]>

* Update pkg/netpol/eval/internal/k8s/adminnetpol.go

Co-authored-by: Tanya <[email protected]>

* Update pkg/netpol/eval/resources.go

Co-authored-by: Tanya <[email protected]>

* Update pkg/netpol/eval/internal/k8s/policy_connections.go

Co-authored-by: Tanya <[email protected]>

* some fixes + a new test

* tiny doc update

* demo test

* tiny change to getPoliciesSelectingPod func and deleting the "deprecated" if statements in "getAllAllowedXgressConnsFromNetpols"

* removing redundant if statements

* new parsed tests with expected outputs and a fix to the func computing "intersection" between ANP's  egress-ingress

* fixing implementing approach + some more parsed tests

* tiny doc update

* eval command support + unit tests

* renaming func

* comment changed

* command line tests with anps + updating support for workloads resources

* removing comment

* changing const names

* fixing if else

* code optimizations and re-org

* moving parsed_resources_tests file + some re-orgs

* adding one more test so all god paths are covered

* optimizing collect from banp + fixing one test output

* optimize + fix + tests confirming results - tested  with policy-assistant

* deny examples parallel to the allow examples added previously

* switch

* policy conns

* collect from banp

* updating outputs with empty line at eof

* Update pkg/netpol/eval/internal/k8s/netpol.go

Co-authored-by: Adi Sosnovich <[email protected]>

* add anp_banp_blog_demo example

Signed-off-by: adisos <[email protected]>

* update example

Signed-off-by: adisos <[email protected]>

* tiny fix

* update example - add another workload and ns

Signed-off-by: adisos <[email protected]>

* first fixes

* common code

* revert accepting workloads as input for evaluate cmd-line

* update example

Signed-off-by: adisos <[email protected]>

* adding tests from dir to command and eval tests; command-line needs generating pod files

* generating the tmp dir in the project path, since permissions are denied on github to the "general" tmp dir

* fixes to fit github permissions

* min-max priority consts

* comments + changing mode

* renaming struct field

* updating mode fields

* rename func

* moving consts

* tmp dir

* renaming attributes

* modifying func

* renaming some tests + adding blog_test to the connlist_test

* test updates

* updating test

* adding references

* updating test anp_test_6_swapping_rules

* test update

* test update

* add test details

Signed-off-by: adisos <[email protected]>

* fix test fail because of a new md file in the test dir - copying only yaml files

---------

Signed-off-by: adisos <[email protected]>
Co-authored-by: Tanya <[email protected]>
Co-authored-by: Adi Sosnovich <[email protected]>
Co-authored-by: adisos <[email protected]>
shireenf-ibm added a commit that referenced this pull request Dec 1, 2024
* adding ANP to parser.k8sobj

* fixing gocritic rangeValCopy by indexing

* w.i.p. anp support - first commit

* more examples (2 ANPs/ ANP+NP)

* fixing references

* new_test that ensures rule ordering in ANP is respected

* update the conn representation as complement in case it is shorter (all but: udp 5353 instead of SCTP 1-65535,TCP 1-65535,UDP 1-5352,5354-65535)

* test with swapped rules from another test + diff test

* more-tests

* fixing conns computations and a test with multiple ANPs

* extending output formats of existing tests

* tiny fix

* fixing a tinu bug in ruleConnections func

* tiny doc updte

* tiny doc update

* a @todo tbd while review

* return error if ANPs are without name or not unique names

* remove redundant lines

* reverting the changes adding complement string representation (all but) for connectionSet

* Merge github.com:np-guard/netpol-analyzer into support_admin_netpolicy

* minor updates to netpol_errors

* currently disabling exposure-analysis when there are admin-network-policies in the input resources

* some organizations (mainly comments updates)

* updating some todo messages

* updating some todo messages/questions

* todo question

* removing a todo that had an answer for, will add some tests on that case

* fixing single anp conns compute when ingress and egress are intersected (not fully matched)

* Update pkg/internal/netpolerrors/netpol_errors.go

Co-authored-by: Tanya <[email protected]>

* Update pkg/netpol/eval/internal/k8s/adminnetpol.go

Co-authored-by: Tanya <[email protected]>

* update todo msg

* some fixes to anp so it matches latest apis

* fixing port-set union func

* Update pkg/netpol/connlist/connlist.go

Co-authored-by: Adi Sosnovich <[email protected]>

* Update pkg/netpol/eval/internal/k8s/adminnetpol.go

Co-authored-by: Adi Sosnovich <[email protected]>

* Update pkg/netpol/internal/common/connectionset.go

Co-authored-by: Adi Sosnovich <[email protected]>

* Update pkg/netpol/eval/internal/k8s/adminnetpol.go

Co-authored-by: Adi Sosnovich <[email protected]>

* Update pkg/netpol/eval/internal/k8s/adminnetpol.go

Co-authored-by: Adi Sosnovich <[email protected]>

* go.mod + lint fix

* adding todo comment

* fixes in subtract

* one line func eliminated

* uniqueness names are required for netpols and admin-netpols

* hasNetpols considers ANPs too

* Tests for AdminNetworkPolicy (#388)

* Added some ANP tests from policy-assistant.
Fixed a small bug in handling named ports in ANP

* fixing lint errors

* Fixing lint error

* Reorganized testing infrastructure from for tests fro parsed resources - creating pod and namespace resources per test; reading expected results from file.
Added more tests from policy assistant.

* fixing lint errors

* return error if ANPs are without name or not unique names

* Revert "return error if ANPs are without name or not unique names"

This reverts commit 1805549.

* Added ANP/BANP names in tests.
Added more tests, including BANP tests, currently commented out.

* Fixed lint errors.

* Fixed lint errors

* Added eval parsed resources tests (along with connlist tests).
Moved all parsed resources tests to a separate file.

* fixing lint errors

* fixing lint errors

* Added testing of CheckIfAllowed and CheckIfAllowedNew

* fixing lint errors

* making linter happy

* Reorganized eval ANP tests, to not depend on connlist.

* Small fixes.

* small fixes

* Changed expected results to not use "all but" expressions.

* making linter happy

* making linter happy

* making lint happy

* making linter happy

* make linter happy

* Creating k8sObjects during a test run, rather then in a test creation.

* making lint happy

* make lint happy

* linter

* shutting up linter

* Moved to parsed_resources_tests some functions used only there.

* Added fake pod status IP fields

* Avoiding unnecessary exports;
Fixing lint errors.

* Making linter happy

* Update pkg/internal/testutils/parsed_resources_tests.go

Co-authored-by: Adi Sosnovich <[email protected]>

* Update pkg/internal/testutils/parsed_resources_tests.go

Co-authored-by: Adi Sosnovich <[email protected]>

* Fixed typos;
removed unneeded change.

---------

Co-authored-by: shireenf-ibm <[email protected]>
Co-authored-by: Adi Sosnovich <[email protected]>

* updating some todo comment which were updated in BANP PR

* sort anps only once before allowed-conns computes (#402)

* sort anps only once before allowed-conns computes

* support_banp (#403)

* support_banp+tests

* removing lint note

* fix merge errors

* why failed to use generics for duplicated code in egressRuleSelectsPeer and ingressRuleSelectsPeer

* banp tests with swapped rules

* integrating Tanya's tests with BANP + adding results; results were compared to policy-assistant, all good

* pass action is not defined for BANP

* more code enhancement, + could not use generics

* adding banp to policy kinds

* adding comment on priority range

* Update pkg/internal/netpolerrors/netpol_errors.go

Co-authored-by: Tanya <[email protected]>

* Update pkg/netpol/eval/internal/k8s/adminnetpol.go

Co-authored-by: Tanya <[email protected]>

* Update pkg/netpol/eval/internal/k8s/adminnetpol.go

Co-authored-by: Tanya <[email protected]>

* Update pkg/netpol/eval/resources.go

Co-authored-by: Tanya <[email protected]>

* Update pkg/netpol/eval/internal/k8s/policy_connections.go

Co-authored-by: Tanya <[email protected]>

* some fixes + a new test

* tiny doc update

* demo test

* tiny change to getPoliciesSelectingPod func and deleting the "deprecated" if statements in "getAllAllowedXgressConnsFromNetpols"

* removing redundant if statements

* new parsed tests with expected outputs and a fix to the func computing "intersection" between ANP's  egress-ingress

* fixing implementing approach + some more parsed tests

* tiny doc update

* renaming func

* comment changed

* removing comment

* changing const names

* fixing if else

* code optimizations and re-org

* moving parsed_resources_tests file + some re-orgs

* optimizing collect from banp + fixing one test output

* optimize + fix + tests confirming results - tested  with policy-assistant

* deny examples parallel to the allow examples added previously

* switch

* policy conns

* collect from banp

* updating outputs with empty line at eof

* adding bad path tests

* add anp_banp_blog_demo example

Signed-off-by: adisos <[email protected]>

* update example

Signed-off-by: adisos <[email protected]>

* tiny fix

* update example - add another workload and ns

Signed-off-by: adisos <[email protected]>

* update example

Signed-off-by: adisos <[email protected]>

* min-max priority consts

* moving consts

* renaming some tests + adding blog_test to the connlist_test

* test updates

* updating test

* adding references

* updating test anp_test_6_swapping_rules

* test update

* test update

* add test details

Signed-off-by: adisos <[email protected]>

---------

Signed-off-by: adisos <[email protected]>
Co-authored-by: Tanya <[email protected]>
Co-authored-by: Adi Sosnovich <[email protected]>
Co-authored-by: adisos <[email protected]>
shireenf-ibm added a commit that referenced this pull request Dec 11, 2024
* adding ANP to parser.k8sobj

* fixing gocritic rangeValCopy by indexing

* w.i.p. anp support - first commit

* more examples (2 ANPs/ ANP+NP)

* fixing references

* new_test that ensures rule ordering in ANP is respected

* update the conn representation as complement in case it is shorter (all but: udp 5353 instead of SCTP 1-65535,TCP 1-65535,UDP 1-5352,5354-65535)

* test with swapped rules from another test + diff test

* more-tests

* fixing conns computations and a test with multiple ANPs

* extending output formats of existing tests

* tiny fix

* fixing a tinu bug in ruleConnections func

* tiny doc updte

* tiny doc update

* a @todo tbd while review

* return error if ANPs are without name or not unique names

* remove redundant lines

* reverting the changes adding complement string representation (all but) for connectionSet

* Merge github.com:np-guard/netpol-analyzer into support_admin_netpolicy

* minor updates to netpol_errors

* currently disabling exposure-analysis when there are admin-network-policies in the input resources

* some organizations (mainly comments updates)

* updating some todo messages

* updating some todo messages/questions

* todo question

* removing a todo that had an answer for, will add some tests on that case

* fixing single anp conns compute when ingress and egress are intersected (not fully matched)

* Update pkg/internal/netpolerrors/netpol_errors.go

Co-authored-by: Tanya <[email protected]>

* Update pkg/netpol/eval/internal/k8s/adminnetpol.go

Co-authored-by: Tanya <[email protected]>

* update todo msg

* some fixes to anp so it matches latest apis

* fixing port-set union func

* Update pkg/netpol/connlist/connlist.go

Co-authored-by: Adi Sosnovich <[email protected]>

* Update pkg/netpol/eval/internal/k8s/adminnetpol.go

Co-authored-by: Adi Sosnovich <[email protected]>

* Update pkg/netpol/internal/common/connectionset.go

Co-authored-by: Adi Sosnovich <[email protected]>

* Update pkg/netpol/eval/internal/k8s/adminnetpol.go

Co-authored-by: Adi Sosnovich <[email protected]>

* Update pkg/netpol/eval/internal/k8s/adminnetpol.go

Co-authored-by: Adi Sosnovich <[email protected]>

* go.mod + lint fix

* adding todo comment

* fixes in subtract

* one line func eliminated

* uniqueness names are required for netpols and admin-netpols

* hasNetpols considers ANPs too

* Tests for AdminNetworkPolicy (#388)

* Added some ANP tests from policy-assistant.
Fixed a small bug in handling named ports in ANP

* fixing lint errors

* Fixing lint error

* Reorganized testing infrastructure from for tests fro parsed resources - creating pod and namespace resources per test; reading expected results from file.
Added more tests from policy assistant.

* fixing lint errors

* return error if ANPs are without name or not unique names

* Revert "return error if ANPs are without name or not unique names"

This reverts commit 1805549.

* Added ANP/BANP names in tests.
Added more tests, including BANP tests, currently commented out.

* Fixed lint errors.

* Fixed lint errors

* Added eval parsed resources tests (along with connlist tests).
Moved all parsed resources tests to a separate file.

* fixing lint errors

* fixing lint errors

* Added testing of CheckIfAllowed and CheckIfAllowedNew

* fixing lint errors

* making linter happy

* Reorganized eval ANP tests, to not depend on connlist.

* Small fixes.

* small fixes

* Changed expected results to not use "all but" expressions.

* making linter happy

* making linter happy

* making lint happy

* making linter happy

* make linter happy

* Creating k8sObjects during a test run, rather then in a test creation.

* making lint happy

* make lint happy

* linter

* shutting up linter

* Moved to parsed_resources_tests some functions used only there.

* Added fake pod status IP fields

* Avoiding unnecessary exports;
Fixing lint errors.

* Making linter happy

* Update pkg/internal/testutils/parsed_resources_tests.go

Co-authored-by: Adi Sosnovich <[email protected]>

* Update pkg/internal/testutils/parsed_resources_tests.go

Co-authored-by: Adi Sosnovich <[email protected]>

* Fixed typos;
removed unneeded change.

---------

Co-authored-by: shireenf-ibm <[email protected]>
Co-authored-by: Adi Sosnovich <[email protected]>

* updating some todo comment which were updated in BANP PR

* sort anps only once before allowed-conns computes (#402)

* sort anps only once before allowed-conns computes

* support_banp (#403)

* support_banp+tests

* removing lint note

* fix merge errors

* why failed to use generics for duplicated code in egressRuleSelectsPeer and ingressRuleSelectsPeer

* banp tests with swapped rules

* integrating Tanya's tests with BANP + adding results; results were compared to policy-assistant, all good

* pass action is not defined for BANP

* more code enhancement, + could not use generics

* adding banp to policy kinds

* adding comment on priority range

* Update pkg/internal/netpolerrors/netpol_errors.go

Co-authored-by: Tanya <[email protected]>

* Update pkg/netpol/eval/internal/k8s/adminnetpol.go

Co-authored-by: Tanya <[email protected]>

* Update pkg/netpol/eval/internal/k8s/adminnetpol.go

Co-authored-by: Tanya <[email protected]>

* Update pkg/netpol/eval/resources.go

Co-authored-by: Tanya <[email protected]>

* Update pkg/netpol/eval/internal/k8s/policy_connections.go

Co-authored-by: Tanya <[email protected]>

* some fixes + a new test

* tiny doc update

* demo test

* tiny change to getPoliciesSelectingPod func and deleting the "deprecated" if statements in "getAllAllowedXgressConnsFromNetpols"

* removing redundant if statements

* new parsed tests with expected outputs and a fix to the func computing "intersection" between ANP's  egress-ingress

* fixing implementing approach + some more parsed tests

* tiny doc update

* renaming func

* comment changed

* removing comment

* changing const names

* fixing if else

* code optimizations and re-org

* moving parsed_resources_tests file + some re-orgs

* optimizing collect from banp + fixing one test output

* optimize + fix + tests confirming results - tested  with policy-assistant

* deny examples parallel to the allow examples added previously

* switch

* policy conns

* collect from banp

* updating outputs with empty line at eof

* add anp_banp_blog_demo example

Signed-off-by: adisos <[email protected]>

* update example

Signed-off-by: adisos <[email protected]>

* tiny fix

* update example - add another workload and ns

Signed-off-by: adisos <[email protected]>

* update example

Signed-off-by: adisos <[email protected]>

* min-max priority consts

* moving consts

* renaming some tests + adding blog_test to the connlist_test

* test updates

* updating test

* adding references

* updating test anp_test_6_swapping_rules

* w.i.p supporting networks (ips) + one goodpath test

* lint fix

* first adding the logger to eval pkg

* adding the logger to k8s policy objects

* adding warnings + tests

* adding unit-tests to check expected warnings are written to the logger

* tiny update to avoid unexpected error of peerStr compare

* test update

* test update

* add test details

Signed-off-by: adisos <[email protected]>

* switching sections of adminnetpol.go file

* adding options to policy-engine without breaking the API

* min&max port

* warning consistency + doc update on IPv6

* enhance test

* test : netpol with empty port range

* test anp with named-port-with-ips

* illegal port range errors

* - logging policy warnings - each warning once
- add warnings to funcs called by eval cmd too

* separating computation per direction to avoid unnecessary lines of disjoint ip-blocks

* removing comment

* shorten warnings

* Update pkg/netpol/eval/internal/k8s/adminnetpol.go

Co-authored-by: Adi Sosnovich <[email protected]>

* adding objects as option to the policy-engine

* moving logPolicyWarnings to checkIfAllowed

* renaming a func

* adding new func GetWorkloadPeersList

* initiating policyengine with objects considering all options

* avoid dup.

* adding warnings type

* returning only workloads as []Peer for connlist API

* Update pkg/netpol/connlist/connlist.go

Co-authored-by: Adi Sosnovich <[email protected]>

* doc  update

---------

Signed-off-by: adisos <[email protected]>
Co-authored-by: Tanya <[email protected]>
Co-authored-by: Adi Sosnovich <[email protected]>
Co-authored-by: adisos <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants